Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/README.md (1)

13-30: ⚠️ Potential issue | 🟡 Minor

Still missing a language on the diagram fence.

Line 13 still opens the state-diagram block without a language, so markdownlint MD040 will keep firing.

✏️ Suggested fix

-```
+```text

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/README.md` around
lines 13 - 30, The code block containing the ASCII state diagram in README.md is
missing a language specifier; update the opening fence (the triple backticks
that start the ASCII diagram block) to include a language such as text (i.e.,
change ``` to ```text) so markdownlint MD040 stops flagging it and the diagram
renders correctly.

ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt (3)

329-335: ⚠️ Potential issue | 🔴 Critical

Still reading instances outside instancesMutex.

Line 330 performs an unsynchronized read from a shared MutableMap before acquiring instancesMutex. Since dynamic circuit creation writes to the same map inside the lock, this is still a real race; please move the whole lookup into the critical section.

🔒 Suggested fix

     suspend fun getInstance(name: String): CircuitBreakerInstance {
-        instances[name]?.let { return it }
         return instancesMutex.withLock {
             instances.getOrPut(name) {
                 CircuitBreakerInstance(name, config.globalConfig, timeSource)
             }
         }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt`
around lines 329 - 335, The current getInstance reads the shared MutableMap
instances outside the instancesMutex, causing a race; move the entire
lookup/creation logic into the critical section by removing the early
unsynchronized instances[name] check and performing instances.getOrPut(name)
inside instancesMutex.withLock so that all accesses to instances (read or write)
happen under instancesMutex; keep creating the CircuitBreakerInstance(name,
config.globalConfig, timeSource) as the getOrPut default.

---

351-355: ⚠️ Potential issue | 🟠 Major

Still counting cancellations as downstream failures.

Line 353 catches every Throwable, and Line 354 records it as a circuit failure. That still includes coroutine cancellation, so caller-aborted requests can open the breaker even when the service is healthy. Re-throw CancellationException before recordFailure(), add a regression test for a locally cancelled request, and tighten the README wording so it does not imply that all exceptions count.

🛠️ Suggested fix

         val call = try {
             proceed(request)
         } catch (cause: Throwable) {
+            if (cause is CancellationException) throw cause
             instance.recordFailure()
             throw cause
         }

This checks the current catch block, confirms how CancellationException is defined in the repo, and shows whether this plugin already has cancellation-focused coverage. Expect the catch block to lack a cancellation guard and the test search to return no dedicated regression case.

#!/bin/bash
set -euo pipefail

FILE='ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt'
TEST_FILE='ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/test/io/ktor/client/plugins/circuitbreaker/CircuitBreakerTest.kt'

echo "== catch block in CircuitBreaker plugin =="
sed -n '349,356p' "$FILE"

echo
echo "== repo definitions of CancellationException =="
rg -n --type=kt 'typealias\s+CancellationException\s*=|class\s+CancellationException\b'

echo
echo "== cancellation-related coverage in CircuitBreakerTest =="
rg -n --type=kt 'CancellationException|cancel' "$TEST_FILE"

Based on learnings, io.ktor.utils.io.CancellationException is a typealias for kotlinx.coroutines.CancellationException, so that single type check excludes all coroutine cancellation exceptions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt`
around lines 351 - 355, The catch in the CircuitBreaker around proceed(request)
currently treats every Throwable as a failure; change the handler so that
CancellationException is re-thrown immediately (e.g., in the catch for cause:
Throwable do if (cause is CancellationException) throw cause) before calling
instance.recordFailure() and re-throwing other exceptions; add a regression test
in CircuitBreakerTest that cancels a locally started request and asserts the
breaker does not record/open from that cancellation; and update the README
wording for the circuit breaker to explicitly state that coroutine cancellations
are excluded from failure counts.

---

173-176: ⚠️ Potential issue | 🟡 Minor

Still reporting HALF_OPEN saturation as OPEN.

Line 175 rejects extra half-open probes with CircuitBreakerOpenException, but this branch is saturated HALF_OPEN, not OPEN. That makes the exposed state and retry timing wrong for callers, because another request may be allowed as soon as a probe finishes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt`
around lines 173 - 176, The code currently throws CircuitBreakerOpenException
when halfOpenAttempts >= config.halfOpenRequests inside the
CircuitState.HALF_OPEN branch; change this to surface the correct HALF_OPEN
condition: replace the thrown CircuitBreakerOpenException(name,
config.resetTimeout) with a distinct exception (e.g.,
CircuitBreakerHalfOpenException) that indicates the circuit is saturated in
HALF_OPEN (or extend CircuitBreakerOpenException to accept a state parameter and
pass CircuitState.HALF_OPEN). Update any callers/tests to expect the new
exception type or check the returned state accordingly; reference
CircuitState.HALF_OPEN, halfOpenAttempts, config.halfOpenRequests, and the
thrown exception in your changes.

ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt (1)

267-277: Add the standard feedback link to this public extension KDoc.

HttpRequestBuilder.circuitBreaker is public API, but its KDoc stops after the description. Please add the repo’s usual [Report a problem] footer so the generated docs stay consistent with other public extensions. Based on learnings, public API extension functions in this repo should include a brief description and a [Report a problem] link.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt`
around lines 267 - 277, The public extension function
HttpRequestBuilder.circuitBreaker is missing the repo's standard "[Report a
problem]" KDoc footer; update the KDoc for circuitBreaker (the block above the
function that references CircuitBreakerConfig.register and
CircuitBreakerConfig.global) to append the standard Report a problem link/footer
used across public APIs so generated docs match other extensions, keeping
existing description and references to CircuitBreakerNameKey unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/README.md (1)

13-13: ⚠️ Potential issue | 🟡 Minor

Add text to the ASCII-diagram fence.

The opening fence at Line 13 still trips markdownlint MD040 without a language tag.

✏️ Proposed fix

-```
+```text

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/README.md` at
line 13, The markdown code fence in the ASCII diagram uses a bare
triple-backtick opening fence which triggers markdownlint MD040; update the
opening fence to include a language tag by replacing the opening "```" with
"```text" so the ASCII-diagram block has a language specified (e.g., change the
fence at the ASCII-diagram opening to "```text").

ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt (2)

173-176: ⚠️ Potential issue | 🟡 Minor

Use a HALF_OPEN-specific rejection here.

Line 175 is rejecting because the half-open probe budget is full, not because the circuit is OPEN until resetTimeout elapses. Reusing CircuitBreakerOpenException makes the public exception, message, and KDoc inaccurate for callers. A distinct rejection path, or at least a HALF_OPEN-specific message/KDoc, would avoid that.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt`
around lines 173 - 176, The current HALF_OPEN branch throws
CircuitBreakerOpenException which misrepresents why a request is rejected;
change this to a distinct HALF_OPEN-specific rejection (e.g., introduce and
throw CircuitBreakerHalfOpenException or similar) so the public exception,
message and KDoc reflect that the half-open probe budget (halfOpenAttempts >=
config.halfOpenRequests) is exhausted rather than the circuit being open until
config.resetTimeout; update the thrown type, its message to mention half-open
probe limit and adjust any KDoc/comments referencing CircuitState.HALF_OPEN and
resetTimeout accordingly.

---

351-355: ⚠️ Potential issue | 🟠 Major

Don’t count coroutine cancellations as downstream failures.

catch (Throwable) currently calls recordFailure() for caller/scope cancellations too, so a locally aborted request can open the circuit even when the service is healthy. Re-throw CancellationException before the catch-all and add a regression test for it; the README note that exceptions “always” count as failures will need the same carve-out.

🛠️ Proposed fix

         val call = try {
             proceed(request)
+        } catch (cause: CancellationException) {
+            throw cause
         } catch (cause: Throwable) {
             instance.recordFailure()
             throw cause
         }

🔎 Verification script

#!/bin/bash
file='ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt'
test_file='ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/test/io/ktor/client/plugins/circuitbreaker/CircuitBreakerTest.kt'

sed -n '344,356p' "$file"
printf '\n-- cancellation-related matches --\n'
rg -n -C2 'CancellationException|catch \(cause: Throwable\)|recordFailure\(|cancel' "$file" "$test_file"

Based on learnings, "In Ktor, io.ktor.utils.io.CancellationException is a typealias for kotlinx.coroutines.CancellationException, which is a supertype of JobCancellationException. Therefore, checking e !is io.ktor.utils.io.CancellationException is sufficient to exclude all coroutine cancellation exceptions."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@ktor-client/ktor-client-plugins/ktor-client-circuit-breaker/common/src/io/ktor/client/plugins/circuitbreaker/CircuitBreaker.kt`
around lines 351 - 355, The catch-all currently treats every Throwable as a
downstream failure by calling instance.recordFailure() inside the catch (cause:
Throwable) block; change the control flow so CancellationException (use
io.ktor.utils.io.CancellationException /
kotlinx.coroutines.CancellationException) is re-thrown before the generic catch
so cancellations do not call instance.recordFailure(), i.e. add an explicit
check for CancellationException and rethrow it (or add a specific catch for it)
and only call instance.recordFailure() for non-cancellation throwables; update
the tests by adding a regression test in CircuitBreakerTest that simulates a
caller/scope cancellation and asserts the circuit is not opened, and adjust the
README note about exceptions counting as failures to exclude coroutine
cancellations.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes